[WIP] Inconsistencies and improvements to SST model #2329

rois1995 · 2024-07-25T13:22:32Z

Proposed Changes

Hi everyone,

I have found some inconsistencies with respect to the literature on the implementation of the Menter's SST model. I would like to use this branch as test bench for any corrections/improvements made to the SST model.

Implementation errors found:

Use of production limiter constant in the clipping of the cross-diffusion term for the computation of the F1 blending function in CTurbSSTVariable.cpp.
SST naming conventions (V in V2003m should stay for Vorticity production term.). Not yet assessed.
Wrong cross-diffusion term included into the residual of w in turb_sources.hpp. It should not be only the positive value as considered in the computation of the F1 blending function. As stated in https://doi.org/10.2514/3.12149. "CDkw is the
positive portion of the cross-diffusion term in Eq (A2)" pag. 1604. Moreover, a clipping has been introduced for large negative values of this term, as suggested in Peng, Shia-Hui, Peter Eliasson, and Lars Davidson. "Examination of the shear stress transport assumption with a low-Reynolds number k-omega model for aerodynamic flows." Eq 17.
Boundary conditions errors at inlets, following the Issue Turbulent Kinetic Energy(TKE) on energy equation in SST model. #1851. A fix has been proposed following @pcarruscag suggestions for the supersonic inlet BC.
Boundary conditions definitions are different than the ones proposed in TMR. Maybe give the user the choice to use the BCs implemented in SU2 or the ones from TMR.

Changes to SST model proposed:

Inclusion of non modified versions of SST. In this case I think that more work is needed to be sure that the correct terms are taken into account everywhere in the code.
Give the user the possibility of changing the production limiter for TKE (not essential).
Change values of default turbulent to laminar viscosity to 1e-2 (NASA TMR reports that it should be in the range of 1e-2 to 1e-5, but in SU2 this is set to 10 as default).

I've seen the proposed changes to the lower limits of k and w in #2323 and I tried implementing it in my branch. However, if the implementation proposed in the respective PR is preferred then I will change mine.

Related Work

#2323 #1851

PR Checklist

Put an X by all that apply. You can fill this out after submitting the PR. If you have any questions, don't hesitate to ask! We want to help. These are a guide for you to know what the reviewers will be looking for in your contribution.

I am submitting my contribution to the develop branch.
My contribution generates no new compiler warnings (try with --warnlevel=3 when using meson).
My contribution is commented and consistent with SU2 style (https://su2code.github.io/docs_v7/Style-Guide/).
I used the pre-commit hook to prevent dirty commits and used pre-commit run --all to format old commits.
I have added a test case that demonstrates my contribution, if necessary.
I have updated appropriate documentation (Tutorials, Docs Page, config_template.cpp), if necessary.

- Added production terms to SST

- Added user-defined production limiter constant for TKE

- Updated boundary conditions as in TMR page

bigfooted · 2024-07-25T20:07:27Z

Common/include/CConfig.hpp

+  nPrandtl_Lam,                  /*!< \brief Number of species 
+  addDoubleOption("FREESTREAM_TURB2LAMVISCRATIO", TurbIntensityAndViscRatioFreeStream[1], 10.0); Prandtl number. */


Was this intentionally commented, or...? If it is used, I guess this should go to line 872

Suggested change

nPrandtl_Lam, /*!< \brief Number of species

addDoubleOption("FREESTREAM_TURB2LAMVISCRATIO", TurbIntensityAndViscRatioFreeStream[1], 10.0); Prandtl number. */

nPrandtl_Lam, /*!< \brief Number of species laminar Prandtl number. */

addDoubleOption("FREESTREAM_TURB2LAMVISCRATIO", TurbIntensityAndViscRatioFreeStream[1], 10.0); /*!<\brief Freestream mu_turb to mu_lam viscosity ratio */

This is just a wrong copy-paste from me. It should not be there in the first place.

bigfooted · 2024-07-25T20:12:19Z

Great contribution, Thanks @rois1995 !

pcarruscag · 2024-07-25T20:31:18Z

If you are looking into robustness aspects too you should get in touch with @emaberman and @YairMO, seems like they have some good ideas and between the free time of 3 people a lot more can get done :)

YairMO · 2024-07-25T23:23:37Z

Hi,

Regarding the cross-diffusion term (CD) that appears in Omega source term (residual). The SST model (1994/2003) is a high-Reynolds-number model. Namely, It can not predict correctly the sub-layer region (especially the correct profile of the TKE). Therefore, only a positive contribution is required. Moreover, since the SST model was design as a k-w and k-epsilon blending, the CD term "belongs" only to the k-epsilon "branch", that is why the CD term include the factor "1-F1". However, it may happen, that the factor "1-F1" is not a 100% safe guarantee. It may happen that "1-F1" is not zero in region where the CD term is negative (this happen due numerical errors). To avoid such a situation, it is a good idea to clip the CD term with zero. Otherwise, severe numerical robustness issues may rise.
Yes, it is different from Menter publications, but I think that clipping the CD term with zero is completely inline with Menter original idea (that is why, I think, he was including the factor "1-F1". But again the 1-F1 factor is not 100% percent "safe").

- Given option for cross diffusion limiting in W residual

YairMO · 2024-07-26T12:28:05Z

Hi,

The use of an Omega production limiter (about the cross-diffusion term) is correct for low-Reynolds-number (LRN) models (the approach described by Peng et al. is very naive; there are other more rigorous treatments). For high-Reynolds-number (HRN) models, the clipping should be zero, keeping the cross-diffusion term positive; thus, the current implementation is correct.

Indeed, it is not exactly as it appears in Menter's original publication. The factor (1-F1) aimed to promise that the cross-diffusion term will be activated only outside the boundary layer, where it is positive (the cross-diffusion term switches its sign deep inside the boundary layer). This was also recognized by Peng et al. (first paragraph above Eq. 17). However, it may happen that the factor (1-F1)=1 where the cross-diffusion term is negative. Usually, it may happen at the wake, very near the airfoil trailing edge, where the upper and lower boundary layers merge. It is due to the imperfection of the F1 function.

To summarize, the current implementation is correct, and it is perfect for HRN models.

YairMO · 2024-07-26T12:52:22Z

For the sake of clarity, "current implementation" refers to the current treatment of the production code

emaberman · 2024-07-26T13:06:32Z

What YairMO is saying, is that allowing negative cross diffusion values is incorrect for high Reynolds models and should not be an option, this is a fix used for low Reynolds models only

rois1995 · 2024-07-26T13:46:58Z

Hi @YairMO, Hi @emaberman ,

thank you very much for your comments. I haven't found any suggestion in literature to clip to only positive values the cross-diffusion term in the w-equation. I understand that it might be more robust, but it is not the standard implementation of the SST model, which is the first thing that we need to achieve. Only then we can build on top of that to improve the robustness of SU2.

Nevertheless, I tried the SWBLI test case and I compared the results across 6 different combinations:

1- develop branch, no changes
2- develop branch, changes to Supersonic_inlet profile as suggested in #1851
3- my branch, with original CDkw implementation (should give exactly the same result as develop+modified BC)
3- my branch, with original CDkw implementation and using boundary conditions from TMR
4- my branch, with original CDkw implementation and using your suggestions for lower limits for k and w.
5- my branch, allowing negative values of CDkw
6- my branch, allowing negative values of CDkw and using boundary conditions from TMR
7- my branch, allowing negative values of CDkw and using your suggestions for lower limits for k and w.
8- my branch, allowing negative values of CDkw, using boundary conditions from TMR and using your suggestions for lower limits for k and w.

When my branch is used, then the changes to the supersonic inlet BC are already in place.

I haven't achieved convergence with 1, 2 and 3. More precisely, 1 diverged right away (after 30 iterations), while 2 and 3 gave "FGMRES - Orthogonalization Failed" after 900ish iterations.

Here you can see the residuals for the different combinations.

Unfortunately I will be busy with the AIAA Conference next week, thus I don't know how much I will be able to work on this. The next test case will be the 2D airfoil near-wake from TMR.

YairMO · 2024-07-26T14:53:44Z

Hi rois1995,

First of all, enjoy your time in Las Vegas. Any paper that you are presenting?

As for our discussion about the cross-diffusion term, I've emailed the "source" (Menter). I believe he will make it clear.
It may be that he will be able to answer only in a while ...

bigfooted · 2024-10-30T22:28:33Z

Common/include/CConfig.hpp

@@ -865,7 +865,7 @@ class CConfig {
  nMu_Temperature_Ref,           /*!< \brief Number of species reference temperature for Sutherland model. */
  nMu_S,                         /*!< \brief Number of species reference S for Sutherland model. */
  nThermal_Conductivity_Constant,/*!< \brief Number of species constant thermal conductivity. */
-  nPrandtl_Lam,                  /*!< \brief Number of species laminar Prandtl number. */


The original description was correct, for species transport we can have a Prandtl number for each of the species

Yeah I do not recall changing that but for sure it was wrong in my branch. Corrected, thanks!

Common/include/CConfig.hpp

SU2_CFD/src/solvers/CEulerSolver.cpp

+      su2double Omega_Freestream = 10 * ModVel_FreeStream / config->GetLDomain();
+      Tke_FreeStream = Omega_Freestream*(Viscosity_FreeStream*config->GetTurb2LamViscRatio_FreeStream())/Density_FreeStream;
+    } else if (config->GetSSTParsedOptions().sust) {
+      su2double Omega_Freestream = 5*ModVel_FreeStream/config->GetLength_Reynolds();


Co-authored-by: Nijso <[email protected]>

SU2_CFD/include/variables/CVariable.hpp

+   * \param[in] iPoint - Point index.
+   * \return Value of magnitude.
+   */
+  inline virtual su2double GetStrainMag(unsigned long iPoint) const {}


To fix the problem, we need to ensure that all execution paths in the function GetStrainMag return a value of type su2double. Since this is a virtual function, the base class implementation can return a default value, such as 0.0, which is a common practice for numerical functions. This will ensure that the function has a defined return value even if it is not overridden by derived classes.

SU2_CFD/include/variables/CVariable.hpp

+   * \brief Get the entire vector of the rate of strain magnitude.
+   * \return Vector of magnitudes.
+   */
+  inline virtual su2activevector& GetStrainMag() {}


To fix the problem, we need to ensure that the function GetStrainMag returns a value of type su2activevector&. Since this is a virtual function, it is likely that derived classes will provide the actual implementation. However, we still need to provide a valid return value in the base class to avoid undefined behavior.

The best way to fix this issue without changing existing functionality is to return a reference to a static instance of su2activevector. This ensures that the function returns a valid reference, and it avoids the need to modify the function's signature or the class's interface.

…_jacobian

…feature_SSTMod

SU2_CFD/include/numerics/turbulent/turb_sources.hpp

+          // su2double MeanReynoldsStress[3][3];
+          // ComputeStressTensor(nDim, MeanReynoldsStress, PrimVar_Grad_i + idx.Velocity(), Eddy_Viscosity_i, 
+          //                     Density_i, ScalarVar_i[0]);


The best way to fix the problem is to remove the commented-out code entirely. This will make the code cleaner and easier to read, and it will eliminate any potential confusion about the commented-out sections. If the code is needed for future reference, it should be documented separately or added to version control comments.

Remove the commented-out code from lines 869 to 879.

Ensure that the remaining code is properly formatted and that no functionality is lost.

SU2_CFD/include/numerics/turbulent/turb_sources.hpp

+          // for (size_t iDim = 0.0; iDim < nDim; iDim++){
+          //   for (size_t jDim = 0.0; jDim < nDim; jDim++){
+          //     PDTerm += MeanReynoldsStress[iDim][jDim] * PrimVar_Grad_i[idx.Velocity() + iDim][jDim];
+          //   }
+          // }
+          // PDTerm = (-alpha2*PDTerm + alpha3 * epsilon * Density_i)*Mt*Mt;


To fix the problem, we should remove the commented-out code entirely. This will make the code cleaner and easier to read, and it will eliminate any potential confusion about whether the commented-out code is meant to be re-enabled or not.

Locate the commented-out code block starting at line 869 and ending at line 878.

Remove the entire block of commented-out code.

Ensure that the remaining code is properly formatted and that no functionality is lost.

…SSTMod

rois1995 added 14 commits June 10, 2024 18:44

- Added TKE production limiter constant to config

2a66c54

- Added production terms to SST

- Added Production and Destruction terms of SST to output

7582590

- Added user-defined lower limits of TKE and W

edce8ee

- Added user-defined production limiter constant for TKE

- Changed the Cross-Diffusion term in F1 computations

41c61ca

- Removed obsolete changes to TKE prod limiter

7b11baa

- Modified the config output

9482f51

- Added lower limit changes for SST in config output

38a02d6

- Added wall distance output for mesh adaptation

c3e0d4b

- Added strain magnitude as output

0f7558b

- added production limiter flag to output

3be370d

- Changed the Cross-Diffusion term in the residual of Omega

125fd6d

- Added clipping of the cross-diffusion term in SST residual

91cc3b8

- changed cross diffusion term clipping in Omega residual

7b78756

- Added CDkw and F1 as output

076d471

rois1995 changed the title ~~Inconsistencies and improvements to SST model~~ [WIP] Inconsistencies and improvements to SST model Jul 25, 2024

rois1995 added 3 commits July 25, 2024 15:25

- Fixed Supersonic inlet BC inclusion of TKE

2e73f77

- Corrected Riemann BC for TKE

c662daf

- Fixed more BCs

41e1c16

- Updated boundary conditions as in TMR page

bigfooted added the changelog:fix label Jul 25, 2024

bigfooted reviewed Jul 25, 2024

View reviewed changes

- Fixed bug with Cross diffusion in W residual

e9dfc27

- Given option for cross diffusion limiting in W residual

- changed default for cross diffusion

9a6fc06

Merge branch 'develop' into feature_SSTMod

16cac0a

bigfooted reviewed Oct 30, 2024

View reviewed changes

Common/include/CConfig.hpp Outdated Show resolved Hide resolved

github-advanced-security bot found potential problems Oct 30, 2024

View reviewed changes

rois1995 and others added 7 commits October 31, 2024 09:53

Update Common/include/CConfig.hpp

c4f10fe

Co-authored-by: Nijso <[email protected]>

Merge branch 'develop' into feature_SSTMod

6eb02da

changes to MLPCpp

f0e280a

Merge branch 'develop' into feature_SSTMod

3ca65bf

- Fix output GetStrainMag bug

32a6eea

- Try fixing output bug again

89a559a

same fix

d4ee84b

github-advanced-security bot found potential problems Mar 4, 2025

View reviewed changes

pcarruscag and others added 10 commits April 3, 2025 19:12

go back to Jacobian modification but fix order of matrix multiplication

4162c72

something else we dont need?

8a1b3da

update serial

0e1d5f9

pressure forces

b2ae9e7

Merge remote-tracking branch 'origin/symmetry_jacobian' into symmetry…

ac30740

…_jacobian

some updates and codeql

5fc0979

more updates

f81e979

- Added Shuzen and Hoffmann compressibility correction

5a40f57

Merge branch 'feature_SSTMod' of https://github.com/su2code/SU2 into …

c370f41

…feature_SSTMod

Merge remote-tracking branch 'origin/develop' into feature_SSTMod

e9eb7ad

github-advanced-security bot found potential problems Apr 10, 2025

View reviewed changes

rois1995 added 7 commits April 10, 2025 15:53

- changed output for debug variables

e8d4bf3

- Added term to jacobian of w

4d2aba5

- Add neighbor wall distance as debug output

e4ceb0e

- Remove Jacobian changes

0d75465

- Add grad vel as output

f159412

- add upper bound into BCs

939faa1

Merge remote-tracking branch 'origin/symmetry_jacobian' into feature_…

99ac333

…SSTMod

		nPrandtl_Lam, /*!< \brief Number of species
		addDoubleOption("FREESTREAM_TURB2LAMVISCRATIO", TurbIntensityAndViscRatioFreeStream[1], 10.0); Prandtl number. */

@@ -1393,3 +1393,3 @@
                */
-              inline virtual su2double GetStrainMag(unsigned long iPoint) const {}
+              inline virtual su2double GetStrainMag(unsigned long iPoint) const { return 0.0; }
@@ -1399,3 +1399,3 @@
                */
-              inline virtual su2activevector& GetStrainMag() {}
+              inline virtual su2activevector& GetStrainMag() { static su2activevector default_vector; return default_vector; }

@@ -1399,4 +1399,6 @@
                */
-              inline virtual su2activevector& GetStrainMag() {}
+              inline virtual su2activevector& GetStrainMag() {
+                  static su2activevector dummy;
+                  return dummy;
+              }
               /*!

[WIP] Inconsistencies and improvements to SST model #2329

Are you sure you want to change the base?

[WIP] Inconsistencies and improvements to SST model #2329

Conversation

rois1995 commented Jul 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed Changes

Related Work

PR Checklist

Uh oh!

bigfooted Jul 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rois1995 Jul 26, 2024

Choose a reason for hiding this comment

Uh oh!

bigfooted commented Jul 25, 2024

Uh oh!

pcarruscag commented Jul 25, 2024

Uh oh!

YairMO commented Jul 25, 2024

Uh oh!

YairMO commented Jul 26, 2024

Uh oh!

YairMO commented Jul 26, 2024

Uh oh!

emaberman commented Jul 26, 2024

Uh oh!

rois1995 commented Jul 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

YairMO commented Jul 26, 2024

Uh oh!

bigfooted Oct 30, 2024

Choose a reason for hiding this comment

Uh oh!

rois1995 Oct 31, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Check notice

Check failure

Copilot Autofix

Check failure

Copilot Autofix

Check notice

Copilot Autofix

Check notice

Copilot Autofix

Uh oh!

rois1995 commented Jul 25, 2024 •

edited

Loading

bigfooted Jul 25, 2024 •

edited

Loading

rois1995 commented Jul 26, 2024 •

edited

Loading